feat(backend): make MSST separation run for real + ground-truth SDR A/B + licence gate - #144
Conversation
…/B + licence gate Follows PR #141's pluggable separation backend. Confirms the licence position (the campaign's stated prerequisite), makes the MSST runner work end-to-end, and adds a ground-truth-stems SDR mode so Demucs-vs-MSST can be measured on real isolated stems. Default path untouched: ASA_SEPARATION_BACKEND still defaults to demucs; analyze.py's JSON contract is unchanged. Licence gate (previously unconfirmed): - MSST-WebUI code is AGPL-3.0; both registry checkpoints (scnet_4stem, bs_roformer_vocals) are CC-BY-NC-SA-4.0 (NonCommercial). That is a strictly worse posture than the (MUSDB-grey) Demucs incumbent, so the MSST backend is research / NonCommercial-only and NOT promotable to a commercial default -- promotion is licence-gated, not quality-gated. - Durable in-code guardrails: LICENCE GATE notes on _MSST_MODEL_REGISTRY and in requirements-msst.txt. Full map + rationale + A/B results: incorporations/msst-separation-licence-gate-2026-06-05.md msst_separate_runner.py -- 3 integration fixes (never exercised before; PR #141 had no MSST install): - chdir into the checkout (MSSeparator import reads data_backup/webui_config.json via a relative path); caller paths resolved to absolute first so they survive. - drop the injected stdlib logger (MSSeparator relies on MSST's own get_logger with a console_handler attribute); its handler targets stderr and the existing redirect_stdout keeps the stdout JSON contract clean. - sample-rate read: regex fallback for configs carrying !!python/tuple tags. separation_ab.py -- ground-truth reference-set mode (--ref-dir): loads MUSDB-style {mixture,vocals,bass,drums,other}.wav, runs each backend, scores true per-stem SI-SDR + aggregate; warmup-skip for the subprocess MSST backend (it reloads per call). New tests/test_separation_ab.py (loader + aggregation + si_sdr). Existing separation tests stay green. NonCommercial research A/B (5 MUSDB18 test tracks, 7s, CPU, mono gain-aligned SI-SDR -- a preliminary proxy, NOT museval): Demucs 8.08 dB @1.3s vs scnet_4stem 6.22 dB @40.8s (~32x slower on CPU). MSST wins vocals (10.77 vs 8.62) but trails overall; this contradicts published museval SDR, so the short-clip proxy is the limiter -- a full-length re-run is the honest basis for any quality claim. The gate conclusion is unchanged either way: do not promote. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
slittycode
left a comment
slittycode
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
Three scoped changes in one PR: runner bugfixes that make the default-off MSST subprocess actually work end-to-end, a ground-truth reference-set mode for the A/B harness (the thing needed to get a real SDR number), and a licence-gate doc + durable in-code guardrails. Product path is untouched — ASA_SEPARATION_BACKEND still defaults to demucs, analyze.py's JSON contract is unchanged, separation_backend.py gets only comments. CI is still running at review time; changes are off-path enough that a failure here would be unrelated infra noise, not a logic regression in this diff.
Findings
Worth considering (not blocking)
evaluate_reference_track passes warmup=False, repeats=repeats (default 2) to run_backend_separation. warmup=False skips the amortising warm pass, but the timing loop still runs repeats times and takes the min — so the reported runtime is min(2 cold-start runs), not "a single honest cold-start" as the comment says. For MSST the second subprocess invocation benefits from OS page-cache warming on the config/checkpoint files, which shaves a fraction of the per-call cold cost. Not wrong enough to block, just slightly off from the claimed semantics. Easy fix: repeats=1 in that call, or update the comment.
_score_against_known buries meanSiSdrDb at the same dict level as the per-stem entries (pre-existing shape). The new _aggregate_reference reads quality.get("meanSiSdrDb") correctly, and the test fixture matches — no bug — but if a future caller iterates quality.items() expecting only stem names it'll trip over meanSiSdrDb as a key. Worth noting if the shape ever gets serialised more broadly.
Test results
Can't run the suite from this environment (no venv). I verified the three symbols the new test file pulls from pre-existing code: SAMPLE_RATE = 44100 (line 43), _write_stereo_wav (line 90, handles 1-D mono input correctly), si_sdr (line 117, gain-aligned SI-SDR — the assertGreater(si_sdr(ref, ref), 100.0) sanity test is correct because zero-noise → inf). Aggregation math in _aggregate_reference tested against a hand-wired fixture; the means are verifiable by inspection. PR description reports 22 green.
Phase boundary check
Clean. No changes to analyze.py, any analyze_*.py module, or the Phase 1 JSON schema. separation_backend.py is comments-only. No Phase 2/3 files touched.
Generated by Claude Code
1 participant
Summary
Follows #141 (pluggable Demucs↔MSST separation backend). This pass does the three things #141 deferred: confirms the licences, makes the MSST runner actually work end-to-end, and adds a ground-truth-stems SDR mode to the A/B harness so Demucs-vs-MSST is measurable on real isolated stems.
Default path is untouched —
ASA_SEPARATION_BACKENDstill defaults todemucs;analyze.py's JSON contract is unchanged.Licence gate (the headline)
scnet_4stem+bs_roformer_vocalsweightsHDEMUCS_HIGH_MUSDB_PLUS)MSST is a strictly worse licence posture on both axes → research / NonCommercial-only, NOT promotable to a commercial default. Promotion is licence-gated, not quality-gated. Full map + rationale + results:
incorporations/msst-separation-licence-gate-2026-06-05.md. Durable in-code guardrails added to_MSST_MODEL_REGISTRYandrequirements-msst.txt.Runner fixes (3 integration bugs only a real install surfaces)
scripts/msst_separate_runner.pyhad never been run (#141 had no MSST install):MSSeparatorimport readsdata_backup/webui_config.jsonvia a relative path; caller paths resolved to absolute first.MSSeparatorneeds MSST's ownget_logger(with aconsole_handlerattribute); its handler targets stderr, and the existingredirect_stdoutkeeps the stdout JSON contract clean.!!python/tupletags.Ground-truth SDR A/B
separation_ab.pygains a--ref-dirmode: loads MUSDB-style{mixture,vocals,bass,drums,other}.wav, runs each backend, scores true per-stem SI-SDR + aggregate. Newtests/test_separation_ab.py; existing separation tests green.NonCommercial research run (5 MUSDB18 test tracks, 7 s, CPU, mono gain-aligned SI-SDR — a preliminary proxy, NOT museval)
scnet_4stemMSST wins vocals (10.77 vs 8.62) but trails overall and collapses on
other(−0.42). This contradicts published museval SDR (scnet ~9.32 > htdemucs ~9.0) → the short-clip mono proxy is the limiter; a full-length re-run (full MUSDB18 download pending) is the honest basis for any quality claim. Conclusion unchanged either way: do not promote.Not included
.runtime/separation_ab/report.json— git-ignored runtime artifact.Test plan
cd apps/backend && ./venv/bin/python -m unittest tests.test_separation_ab tests.test_separation_backend— 22 green.🤖 Generated with Claude Code